fix(migration): stop the report claiming PASSED when the red step is outside every phase (#1141) - #1142
Merged
Conversation
…outside every phase (#1141) #1120 taught the report to reconcile the runner's `steps.<id>.outcome` against what each phase recorded, so a phase that crashed before writing its verdict can no longer render as `Result: PASSED`. Only three steps carry an `id`, so only three steps were reconciled. The steps BETWEEN them carry none: resolving the nightly version from PyPI, installing it, starting it, and waiting out the alembic migration. A failure there stops the job, so phases 2 and 3 never run and their outcomes arrive empty — `declared_outcomes()` filters those out, the state file still holds a fully-passing `latest` phase, and the report printed `Result: PASSED` into an issue titled "Langflow Migration Test Failed". The nightly failing to boot against a migrated database is exactly what this workflow exists to catch, and it landed in that gap. The workflow now also hands over `JOB_STATUS` (`job.status`, already used by the summary step below it). A job the runner reports as `failure`/`cancelled`, whose report found no failure and no integrity problem, is itself an integrity problem: the failure is real and lives outside every phase the report can see. It is only raised when nothing else attributes the failure — the phase-level message says it better and duplicating it would be noise. Reconciling the job status covers the four steps that exist today and any added later, which per-step `id`s would not. Also: `## Unaccounted phases` is now `## Unaccounted failures`, since the section can hold an entry that belongs to no phase; the header states the runner's verdict next to the report's, the pair that contradicted each other; and `__pycache__/` is ignored — running the Python tests locally left untracked noise in every subsequent diff. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1141. Follow-up to #1120 (PR #1139) — the other half of the same defect.
The gap
#1139 taught
generate_report.pyto reconcile the runner'ssteps.<id>.outcomeagainst what each phase recorded, so a phase that crashed before writing its verdict can no longer render asResult: PASSED. Only three steps carry anid, so only three steps were reconciled.The steps between them carry none:
migration-test.yml:146Resolve nightly version from PyPIexit 1when PyPI returns no pre-releasemigration-test.yml:167Install Langflow nightlymigration-test.yml:181Start Langflow nightly (same database)migration-test.yml:190Wait for Langflow nightly (includes migration)A failure there stops the job, so
Verify migration via API/via UInever run and their outcomes arrive as the empty string.declared_outcomes()filters those out, the state file still holds a fully-passinglatestphase,assess()finds nothing wrong — and the report printedResult: PASSEDinto an issue titled "Langflow Migration Test Failed".The nightly failing to boot against a migrated database is exactly what this workflow exists to catch, and it landed in that gap.
The fix
Generate reportnow also receivesJOB_STATUS: ${{ job.status }}— the same expression the summary step below it already uses. A job the runner reports asfailure/cancelled, whose report found no failure and no integrity problem, is an integrity problem: the failure is real and lives outside every phase the report can see.Two properties, both tested:
fail, or one declaredfailureby the runner, says it better; duplicating it would be noise.PASSED. If the job is red, something failed — a report sayingPASSEDis wrong by definition, so there is no false-positive direction here.Chosen over adding
id+PHASE_OUTCOME_*to the four steps (issue option 2), which covers only today's steps and degrades silently the moment a fifth is added.Also in this PR:
## Unaccounted phases→## Unaccounted failures— the section can now hold an entry belonging to no phase.**Job status (runner):** \failure``) — the pair that contradicted each other across title and body.__pycache__/+*.pycignored: running fix(migration): stop the report reading silence as success, and drive the Playground with real testids (#1120) #1139's Python tests locally left untracked noise in every subsequent diff.Validation
Unit tests — 21 → 27, all green (
python-unitslane, added in #1139):test_a_red_job_with_no_phase_failure_is_not_a_passFAILED+ an entry namingoutside every phaseandRead the job logtest_the_same_state_still_passes_when_the_job_is_greensuccess,None) →PASSED,integrity == []test_a_cancelled_job_is_not_a_passcancelledis not a pass either; the message names ittest_a_red_job_is_not_reported_twice_when_a_phase_already_owns_the_failurefail, and run #115's declared failure, each yield exactly one entry — the phase'stest_job_status_is_parsed_from_the_env"Failure"→failure, whitespace trimmed, unset/blank →None(unknown, never a status)test_the_rendered_report_states_both_verdicts## Result: FAILED+ section, in the rendered markdownForce-fail — 5 mutations, each dropping the intended test:
Reverted afterwards: no mutation markers in the diff,
27 passed.End-to-end through the real env path (the unit tests pass a dict to
declared_outcomes(); CI goes throughos.environ), driving the script exactly as the step does:Counter-check: three phases green +
JOB_STATUS=success→## Result: PASSED.Reproduce locally:
Other gates:
tsc --noEmit0 errors; ESLint 0 errors (warnings pre-existing); the workflow YAML parses and the report step'senvcarries all four variables.Not proven here: that GitHub actually populates
JOB_STATUSat that point in the job — the same residual as #1139'sPHASE_OUTCOME_*, which no local harness can close. Aworkflow_dispatchofmigration-test.ymlproves both at once; what to watch in thePrint reportstep is**Job status (runner):**being present and## Result:agreeing with the job's own status. No spec files are touched, so nothing here creates flows.🤖 Generated with Claude Code